Skip to content

feat: expose ClientTrustConfig as a public class #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SequeI
Copy link
Contributor

@SequeI SequeI commented Aug 8, 2025

Summary

Exposing ClientTrustConfig. Since recent changes to SigningContext.staging/production being moved to ClientTrustConfig, it would be better for this class to be public as it is/will be used in Model-Transparency and such. The API has had massive changes as is, so I think it would be better doing this now for next release.

Resolves #1019

Release Note

Added to changelog.md

The ClientTrustConfig class has been moved from the private _internal package to a new public
module (sigstore.trust). This change formally adds the class to the project's public API,
making it available for use in other projects.

@SequeI SequeI force-pushed the publicClientTrustConfig branch 2 times, most recently from 8ec76df to 692955d Compare August 8, 2025 15:52
@SequeI
Copy link
Contributor Author

SequeI commented Aug 8, 2025

Noticed the rekorV2 tests are wishy washy

@jku
Copy link
Member

jku commented Aug 11, 2025

Noticed the rekorV2 tests are wishy washy

If you mean that the signing tests using staging rekor2 fail with 50x errors more often then is reasonable: yes this seems to be the case. I'm following up in sigstore/rekor-tiles repo

@SequeI SequeI force-pushed the publicClientTrustConfig branch from 692955d to fc1630e Compare August 11, 2025 11:10
@SequeI
Copy link
Contributor Author

SequeI commented Aug 11, 2025

sizeable new main commit; rebased pr

@woodruffw
Copy link
Member

/gcbrun

@woodruffw
Copy link
Member

Thanks @SequeI!

Exposing this as public makes sense to me. Two thoughts:

  • What do you think about exposing this via sigstore.models instead? I've tried to centralize more of our data models there, especially when shared between signing and verification. The upside is fewer top-level modules in the sigstore.* namespace; the downside is that module gets chunkier.
  • Is there anything else that we should expose from sigstore._internal.trust? I can't think of anything else off the top of my head, but I figure you'd have a better sense as a downstream user.

@woodruffw woodruffw added enhancement New feature or request component:api Public APIs labels Aug 11, 2025
@SequeI SequeI force-pushed the publicClientTrustConfig branch 3 times, most recently from a7b8e19 to a13895d Compare August 11, 2025 15:23
@SequeI SequeI force-pushed the publicClientTrustConfig branch from a13895d to a700c31 Compare August 11, 2025 15:29
@woodruffw
Copy link
Member

woodruffw commented Aug 11, 2025

@SequeI Could you avoid force-pushing unless absolutely necessary? It causes a notification to me (and presumably anything one else subscribed on the PR) each time you do 🙂

(It's okay to have a non-clean history on this PR -- we'll clean it up with the actual merge.)

@SequeI
Copy link
Contributor Author

SequeI commented Aug 11, 2025

@woodruffw Apologies! Won't do that anymore

Would it make sense to have a new public module (trust like previously) and just expose trustedRoot and SigningConfig alongside ClientTrustConfig due to it's dependence on it? Putting it into models creates a lot of circular import errors with rekor and rekorv2 unfortunately

@woodruffw
Copy link
Member

Putting it into models creates a lot of circular import errors with rekor and rekorv2 unfortunately

Hmm, could you paste those? I think we'd ideally eliminate those potential circularities are part of v4 anyways; they should all be refactorable.

@SequeI
Copy link
Contributor Author

SequeI commented Aug 11, 2025

Sure :

    from sigstore._internal.rekor.client import RekorClient
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
../../.local/lib/python3.13/site-packages/sigstore/models.py:65: in <module>
    from sigstore._internal.trust import SigningConfig, TrustedRoot
../../.local/lib/python3.13/site-packages/sigstore/_internal/trust.py:43: in <module>
    from sigstore._internal.rekor.client_v2 import RekorV2Client
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client_v2.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
E   ImportError: cannot import name 'TransparencyLogEntry' from partially initialized module 'sigstore.models' (most likely due to a circular import) (/home/.local/lib/python3.13/site-packages/sigstore/models.py)
test/unit/conftest.py:40: in <module>
    from sigstore._internal.rekor.client import RekorClient
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
../../.local/lib/python3.13/site-packages/sigstore/models.py:65: in <module>
    from sigstore._internal.trust import SigningConfig, TrustedRoot
../../.local/lib/python3.13/site-packages/sigstore/_internal/trust.py:44: in <module>
    from sigstore._internal.rekor.client import RekorClient
E   ImportError: cannot import name 'RekorClient' from partially initialized module 'sigstore._internal.rekor.client' (most likely due to a circular import) (/home/.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py)

I wasn't sure if this would be in the scope of the PR, or if I should fix this etc, but I'll fix these in this case

@woodruffw
Copy link
Member

I wasn't sure if this would be in the scope of the PR, or if I should fix this etc, but I'll fix these in this case

Thank you! Yes, I'd consider it in scope, and I appreciate you fixing them 🙂

Signed-off-by: SequeI <[email protected]>
@SequeI
Copy link
Contributor Author

SequeI commented Aug 11, 2025

Resolved; I had thought it would be much more involved but was just a small issue with RekorV1/V2 placement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api Public APIs enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public API: Expose ClientTrustConfig
3 participants